Skip to content

Comments

Fix: Prevent game crash on sink double-click#56

Open
Itx-Psycho0 wants to merge 3 commits intoknative-extensions:mainfrom
Itx-Psycho0:fix-sink-double-click-crash
Open

Fix: Prevent game crash on sink double-click#56
Itx-Psycho0 wants to merge 3 commits intoknative-extensions:mainfrom
Itx-Psycho0:fix-sink-double-click-crash

Conversation

@Itx-Psycho0
Copy link
Contributor

Changes

  • 🐛 Fix bug: Added a safety check in the transfer_box function.
  • The code now checks if conveyerInd is within the valid range of the conveyer array before accessing it.
  • This prevents the "Invalid access to index" crash when a user double-clicks the Sink.

/kind bug

Fixes #55

Verification (Video)

issue55.mp4

Release Note

Fixed a critical bug where the game would crash if the user double-clicked the Sink container.

@knative-prow
Copy link

knative-prow bot commented Jan 26, 2026

@Itx-Psycho0: The label(s) kind/bug cannot be applied, because the repository doesn't have them.

Details

In response to this:

Changes

  • 🐛 Fix bug: Added a safety check in the transfer_box function.
  • The code now checks if conveyerInd is within the valid range of the conveyer array before accessing it.
  • This prevents the "Invalid access to index" crash when a user double-clicks the Sink.

/kind bug

Fixes #55

Verification (Video)

issue55.mp4

Release Note

Fixed a critical bug where the game would crash if the user double-clicked the Sink container.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jan 26, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: Itx-Psycho0 / name: Anurag Singh (4926e6d)

@knative-prow
Copy link

knative-prow bot commented Jan 26, 2026

Welcome @Itx-Psycho0! It looks like this is your first PR to knative-extensions/educational-game 🎉

@knative-prow knative-prow bot added the size/XS label Jan 26, 2026
@Itx-Psycho0
Copy link
Contributor Author

Hi @ankitajana21 @prajjwalyd,

I have signed the CLA and verified the fix locally. The double-click crash issue is resolved.

Summary of Changes:

Added safety check for conveyerInd array bounds.

Verified with the attached video proof.

Ready for your review!

@Cali0707 Cali0707 requested review from ankitajana21 and prajjwalyd and removed request for mmejia02 and zainabhusain227 January 26, 2026 16:59
Copy link
Member

@prajjwalyd prajjwalyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this fix @Itx-Psycho0! The check prevents the crash.

@knative-prow
Copy link

knative-prow bot commented Jan 27, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Itx-Psycho0, prajjwalyd
Once this PR has been reviewed and has the lgtm label, please assign cali0707 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow knative-prow bot added the lgtm label Jan 27, 2026
@prajjwalyd
Copy link
Member

One small concern, though:
do we need to handle the case where a player clicks the same sink more than once in levels with multiple sinks cc @ankitajana21?

@ankitajana21
Copy link
Member

@prajjwalyd I'll review this PR by today, the script is common to all levels, so I have to check it.

@ankitajana21
Copy link
Member

@Itx-Psycho0 This is not working for multi sink level, let's say I click once on blue, and twice on red, and after that I can't click on green, as it is considering 2 belts for red and not allowing the 3rd one for green. FYI, In the multi sink level, the player need not click all the sinks, the player can click less as well. The events are duplicated accordingly for the number of sinks connected.

@ankitajana21
Copy link
Member

/lgtm cancel

…ion bug

- Added dynamic conveyor creation for MultiSink level (3+ conveyors)
- Maintained safety check for BasicEventFlow to prevent crash
- Fixed level.gd to initialize before scene change (not after)
- Resolves issue where MultiSink failed during full game playthrough
- Addresses maintainer feedback about connection limits

Fixes knative-extensions#55
@knative-prow knative-prow bot added size/S and removed size/XS labels Jan 29, 2026
@Itx-Psycho0
Copy link
Contributor Author

Hi @ankitajana21,

Thank you for the detailed feedback! You were absolutely right - my initial fix limited MultiSink to 3 connections. I've now implemented a comprehensive solution that fixes both the crash AND allows unlimited connections.

Root Cause

After debugging, I found TWO bugs:

  1. Original bug: Index out of bounds on double-click (Issue [Bug] Game crashes with "Invalid access to index" on double-clicking the Sink #55)
  2. Hidden bug: Scene transition order in level.gd was clearing the events array AFTER boxes added themselves, causing MultiSink to fail when playing through the full game (though it worked when running directly with F6)

Solution

ConveyerController.gd - Smart Conveyor Management

if conveyerInd >= conveyer.size():
    // Detect scene by conveyor count
    if conveyer.size() >= 3:
        // MultiSink: Create new conveyors dynamically
        var new_conveyor = conveyer[0].duplicate()
        get_tree().current_scene.add_child(new_conveyor)
        conveyer.append(new_conveyor)
    else:
        // BasicEventFlow: Stop to prevent crash
        return

level.gd - Fixed Scene Transition

// Initialize BEFORE changing scene (not after)
ConveyerController.initialise()
get_tree().change_scene_to_file(next_level_path)

Testing Results

BasicEventFlow: Double-click no longer crashes
BoxClick: Works correctly
MultiSink - Your test case: Blue once + Red twice + Green once = SUCCESS
MultiSink - Unlimited: Can now make 5+ connections
Full playthrough: All levels work in sequence

Video Demonstration

Screen.Recording.2026-01-29.102927.mp4

The fix now properly addresses both the crash prevention AND allows the flexible multi-sink behavior you described. Ready for review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Game crashes with "Invalid access to index" on double-clicking the Sink

3 participants